Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/readme migration v6 #1297

Merged
merged 11 commits into from
Oct 25, 2023
Merged

feature/readme migration v6 #1297

merged 11 commits into from
Oct 25, 2023

Conversation

lavigne958
Copy link
Collaborator

@lavigne958 lavigne958 commented Sep 10, 2023

  • Add HTTP client migration for v6
  • No more python3.7
  • update method signature change
  • New usage of hex color for tab colors

I added a whole new section in the docs that we can keep for some time.

It was way too big to be only in the README, it took too much space.

We can keep pushing on this branch until we are complete. what do you think @alifeee ?

Wtb at the end we need to add a link to the documentation in the README too but we can keep that for the last commit

@lavigne958 lavigne958 self-assigned this Sep 10, 2023
@alifeee
Copy link
Collaborator

alifeee commented Sep 12, 2023

this is great :)

Since the migration guide is a very "temporary" item: in my eyes, it would go best on the Release page for v6.0.0, (similar to the v5.11.0 release page where I added a note about an exception). We could (and should) still link to it from the readme/docs.

What do you think? If not, I think an .rst documentation file ultimately works fine :)

@alifeee alifeee added this to the 6.0.0 milestone Sep 12, 2023
@alifeee alifeee self-assigned this Sep 12, 2023
@alifeee alifeee marked this pull request as draft September 12, 2023 20:26
@alifeee alifeee added the Docs label Sep 12, 2023
@lavigne958
Copy link
Collaborator Author

I started writing in the README but it started to be so long I though it deserve its own space in the doc. We can keep it we never know when someone will upgrade to version 6, now or in a couple of years.

In the readme I pan to add some section that has a link to the doc for the v6 migration

@lavigne958
Copy link
Collaborator Author

I thought about it for some time, maybe we should keep it in the README which will force us to make it clear and short as well.

you think it's a good idea ?

@alifeee
Copy link
Collaborator

alifeee commented Oct 17, 2023

I thought about it for some time, maybe we should keep it in the README which will force us to make it clear and short as well.

That sounds like a good idea

Signed-off-by: Alexandre Lavigne <[email protected]>
Signed-off-by: Alexandre Lavigne <[email protected]>
Signed-off-by: Alexandre Lavigne <[email protected]>
Signed-off-by: Alexandre Lavigne <[email protected]>
@lavigne958 lavigne958 force-pushed the feature/readme_migration_v6 branch from 82bc30b to 056eb28 Compare October 24, 2023 12:22
@lavigne958
Copy link
Collaborator Author

rebased and updated the code so the migration guide is located in the README.md

@lavigne958 lavigne958 requested a review from alifeee October 24, 2023 12:23
@lavigne958 lavigne958 marked this pull request as ready for review October 24, 2023 12:24
Copy link
Collaborator

@alifeee alifeee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good so far. I think we leave this open until we are ready to release 6.0.0 and then we can look at merging it.

I think the v6.0.0 migration section should go between the Basic Usage and More Examples section. We should prioritise new users by showing the basics at the top of the README.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lavigne958
Copy link
Collaborator Author

This is good so far. I think we leave this open until we are ready to release 6.0.0 and then we can look at merging it.

I disagree with this, we should merge this when it's ready so users can check it out, start planning and start updating what can be done. the best way would be: the users are ready to upgrade, then we make the new release and users can just merge their ready to go PR that includes code change and the bump of gspread version.

because: a lot of users will be late 😆

I think the v6.0.0 migration section should go between the Basic Usage and More Examples section. We should prioritise new users by showing the basics at the top of the README.

agreed, make sense to show first the basic usage for the new users. 👍

I will update the doc with your suggestions

@alifeee
Copy link
Collaborator

alifeee commented Oct 25, 2023

we should merge this when it's ready so users can check it out, start planning and start updating what can be done

agreed.

I have made some changes, in effort to make the guide section of the README as small as possible. You can see further comments in #1279 (comment)

I think that now we can merge this, and if we think of new breaking changes that should be added to the README, we can create a new PR linked to #1297

@lavigne958 lavigne958 merged commit 2bddb93 into master Oct 25, 2023
12 checks passed
@lavigne958 lavigne958 deleted the feature/readme_migration_v6 branch October 25, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants